CSV panel actions for SavedSearch embedabbles #33709

joelgriffith · 2019-03-22T15:14:59Z

Retry of #32522 since this PR wasn't showing the appropriate deltas.

joelgriffith · 2019-03-22T15:17:45Z

x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx

+
+  public isVisible = (panelActionAPI: PanelActionAPI): boolean => {
+    const { embeddable } = panelActionAPI;
+


Had to go with duck-type-typing here since it was proving to be impossible to import the SavedSearchEmbedable component :/

Ok, so the actual class we want is from src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts, which is an extension of the Embeddable class. That's why we have to check for the savedSearch property to make sure it's valid?

In that case, I think this could use a comment to marker where we might be able to improve things down the road.

elasticmachine · 2019-03-22T16:01:04Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

joelgriffith · 2019-03-22T16:18:17Z

src/legacy/core_plugins/kibana/public/dashboard/store/panel_actions_store.ts

@@ -28,7 +28,9 @@ class PanelActionsStore {
   */
  public initializeFromRegistry(panelActionsRegistry: ContextMenuAction[]) {
    panelActionsRegistry.forEach(panelAction => {


Some reason panel actions are duped everytime you re-enter a page when client-side routing occurs. This fixes that.

joelgriffith · 2019-03-22T16:19:12Z

x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx

+
+    await kfetch({ method: 'POST', pathname: `${API_BASE_URL}${id}`, body }, { parseJson: false })
+      .then(r => r.text())
+      .then(csv => {


Kind of a nasty client-side mechanism to do CSV downloads browser-side. Might want to consolidate this into some sort of util elsewhere.

Looks like typical JS to me 😁

src/legacy/ui/public/kfetch/kfetch.ts

tsullivan · 2019-03-25T18:01:18Z

x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx

+import { kfetch } from 'ui/kfetch';
+import { toastNotifications } from 'ui/notify';
+
+const API_BASE_URL = '/api/reporting/v1/generate/immediate/csv/saved-object/';


I think it'd be worthwhile to import the base URL path (API_BASE_URL_V1) from '../../common/constants'

tsullivan · 2019-03-25T18:44:14Z

x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx

+    const state = await this.generateJobParams({ searchEmbeddable });
+
+    const id = `search:${embeddable.savedSearch.id}`;
+    const filename = embeddable.savedSearch.title;


Maybe next, we should work on exposing the SearchEmbeddable interface here, because .savedSearch and .$rootScope are typed as private in src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable

In addition to the other headaches with the SearchEmbeddable interface (😁) it seems too limited for the definition to be in /discover. The Discover app is used for creating a saved search... but having the defined structure of a saved search is important all over the Kibana codebase.

Yeah, I think I'm going to add another issue where we do some cleanup/migration of this component so we can better use it in our x-pack code.

tsullivan

I left a couple of comments, which are really more for discussion. The only things I'm really interested to see changed are the thing about using the constants file, and adding that minor comment.

tsullivan · 2019-03-25T18:49:30Z

x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx

+  };
+
+  private onGenerationFail(error: Error) {
+    toastNotifications.addDanger({


tsullivan · 2019-03-26T21:57:57Z

x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx

+    }
+
+    const searchEmbeddable = embeddable;
+    const state = await this.generateJobParams({ searchEmbeddable });


I think this needs to be wrapped in _.pick to whitelist fields that we care about. For example, we don't care about highlight, size, and script_fields (since they can't create script fields on-the-fly)

Oh, this is just a comment of something we can figure out later, because I'm not sure what we need at the moment.

Another thing (that doesn't need to be figured out right now) - this object doesn't seem to contain the columns.

Create a saved search

Add it to a dashboard

Change the columns in the panel of the dashboard

We need to figure out how to get the columns to show, it's not in this state (but it could be)

Sounds good!

tsullivan · 2019-03-26T22:33:59Z

Will it be possible to not have the download link show up when the dashboard is in edit mode?

elasticmachine · 2019-03-27T22:29:40Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-28T16:49:39Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

tsullivan · 2019-03-28T20:35:34Z

There's a valid test failure of some of our code being wrong after the master merge:

09:22:46 ERROR x-pack/test failed
09:22:46       /var/lib/jenkins/workspace/elastic+kibana+pull-request/JOB/kibana-intake/node/immutable/kibana/x-pack/test/reporting/api/generate/csv_saved_search.ts
09:22:46       ERROR: 7:20  module-migration  Imported module "expect.js" should be "@kbn/expect"

elasticmachine · 2019-03-28T20:51:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

joelgriffith · 2019-03-28T20:54:35Z

I've thrown down some TODO's as well as hidden the export in edit mode. I think that resolves most of the comments here @tsullivan if you want to give it a final glance.

After that we can begin the PR from our feature branch back into master. I don't mind taking ownership of that.

elasticmachine · 2019-03-28T21:37:48Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

tsullivan

LGTM

AlonaNadler · 2019-03-29T22:35:17Z

Congrats on the merge! great job :)

joelgriffith requested review from tsullivan and stacey-gammon March 22, 2019 15:15

joelgriffith mentioned this pull request Mar 22, 2019

Adding CSV panel actions for SavedSearch embedabbles #32522

Closed

joelgriffith commented Mar 22, 2019

View reviewed changes

tsullivan reviewed Mar 25, 2019

View reviewed changes

src/legacy/ui/public/kfetch/kfetch.ts Show resolved Hide resolved

tsullivan reviewed Mar 25, 2019

View reviewed changes

tsullivan requested changes Mar 25, 2019

View reviewed changes

tsullivan changed the title ~~Feature/reporting/csv export panel action ui~~ CSV panel actions for SavedSearch embedabbles Mar 26, 2019

tsullivan reviewed Mar 26, 2019

View reviewed changes

tsullivan self-assigned this Mar 28, 2019

rayafratkina added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Mar 28, 2019

joelgriffith force-pushed the feature/reporting/csv-export-panel-action branch from f43ad9f to 34677f2 Compare March 28, 2019 20:10

Joel Griffith added 7 commits March 28, 2019 13:11

Adding CSV panel actions for SavedSearch embedabbles

d307898

WIP: Implementing PR feedback

0ab1034

Fixing imports, timzeone config, and more for exporting CSV

446f6ce

Duck typing vs instanceof checks

428c831

Moving base URL over to constants

fcda1dc

Spec for kfetch

e826e28

Some placemarkers for things to cleanup once we get SavedSearch updated

30360a8

Don't allow exports in edit mode

d658e75

Fixing expectjs import

4cf525e

tsullivan approved these changes Mar 28, 2019

View reviewed changes

joelgriffith merged commit fb3776c into elastic:feature/reporting/csv-export-panel-action Mar 29, 2019

This was referenced Apr 2, 2019

Hide 'Time' column setting doesn't work with export to CSV #34377

Closed

Export to CSV from a dashboard is not consistent with Discover CSV behavior #34376

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CSV panel actions for SavedSearch embedabbles #33709

CSV panel actions for SavedSearch embedabbles #33709

joelgriffith commented Mar 22, 2019

joelgriffith Mar 22, 2019

tsullivan Mar 25, 2019

elasticmachine commented Mar 22, 2019

joelgriffith Mar 22, 2019

joelgriffith Mar 22, 2019

tsullivan Mar 25, 2019

tsullivan Mar 25, 2019

tsullivan Mar 25, 2019

tsullivan Mar 25, 2019

joelgriffith Mar 28, 2019

tsullivan left a comment

tsullivan Mar 25, 2019

tsullivan Mar 26, 2019

tsullivan Mar 26, 2019

tsullivan Mar 26, 2019

joelgriffith Mar 27, 2019

tsullivan commented Mar 26, 2019

elasticmachine commented Mar 27, 2019

elasticmachine commented Mar 28, 2019

tsullivan commented Mar 28, 2019

elasticmachine commented Mar 28, 2019

joelgriffith commented Mar 28, 2019

elasticmachine commented Mar 28, 2019

tsullivan left a comment

AlonaNadler commented Mar 29, 2019


		public isVisible = (panelActionAPI: PanelActionAPI): boolean => {
		const { embeddable } = panelActionAPI;

CSV panel actions for SavedSearch embedabbles #33709

CSV panel actions for SavedSearch embedabbles #33709

Conversation

joelgriffith commented Mar 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Mar 22, 2019

💚 Build Succeeded

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsullivan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsullivan commented Mar 26, 2019

elasticmachine commented Mar 27, 2019

💔 Build Failed

elasticmachine commented Mar 28, 2019

💔 Build Failed

tsullivan commented Mar 28, 2019

elasticmachine commented Mar 28, 2019

💔 Build Failed

joelgriffith commented Mar 28, 2019

elasticmachine commented Mar 28, 2019

💚 Build Succeeded

tsullivan left a comment

Choose a reason for hiding this comment

AlonaNadler commented Mar 29, 2019